-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port OCaml bindings to use Dune and Opam #2319
Conversation
There is still missing proper testing, I think I will just write a proper tests, that will compare the output with the expected, and will add the testing step to the CI. |
Yes, for now this is enough. But at minimum the build should added in the CI. |
Hi. Against which version of capstone this binding should be compiled? The OCaml stub does not seem in sync with the Capstone API as found in the branch the OCaml stub belongs to. |
The bindings build correctly with Capstone 5.0.1. |
Hi, it's still in progress. I am targeting 6.0. I still need to implement missing architectures support that were added since the last time OCaml bindings were touched. I also plan to add proper testing with alcotest. Moreover, the auto-sync scripts should be extended to auto-regenerate constants so that they are in sync with the C version. Once everything is done, I also plan to package it for opam and send PR to the opam-repository. |
if this is working for 5.0.1, please make PR for branch v5, then we can merge it there. thanks for taking care of this binding! |
I don't really want to spend time on 5.0 for these, since I am working on changing the Python code generators to regenerate constants for newly updated architectures - currently there is a mismatch between those. |
Ok, I thought you wanted that since someone mentioned v5.
…On Thu, Jun 13, 2024, 09:52 Anton Kochkov ***@***.***> wrote:
if this is working for 5.0.1, please make PR for branch v5, then we can
merge it there.
thanks for taking care of this binding!
I don't really want to spend time on 5.0 for these, since I am working on
changing the Python code generators to regenerate constants for newly
updated architectures - currently there is a mismatch between those.
—
Reply to this email directly, view it on GitHub
<#2319 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABNQNYGQQ57RZNNVMHYB2XDZHD3MPAVCNFSM6AAAAABF6A5U42VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRUGIYDGOBRG4>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
@XVilka When you continue to work on this one, please note the minimal You just have to make sure the logging messages are the same as the tests expect and they are passed to The tests should be run like the for Python:
|
@Rot127 I did some updates in the generation script, and also added missing architectures. Now everything compiles just fine. The only missing part now is alcotest and fixes along the way. |
Hi. Should I understand that that PR is finally not going to be merged? I see that the PR repo. has been archived. I am using capstone with OCaml (using local minimal patches so that it compiles and fits my needs). So if the road is clear, I can take over this PR. |
@strub I am sorry, I forgot to add the link to the reason. Please see: https://x.com/akochkov/status/1891844161822691543?s=19 XVilka plans to continue it. Or maybe he is happy to pass it on? |
@strub I decided to use modern way of writing OCaml bindings instead of all this code - by using ctypes and cstubs: https://michael.bacarella.com/2022/02/19/dune-ctypes/ |
Your checklist for this pull request
Detailed description
Switch to use what most modern OCaml projects use - Dune build system: https://dune.build/
Provide an opam package
Based on bindings/ocaml: fix ARM64 -> AArch64 #2318
Test plan
opam install .
dune test
Closing issues
Closes #985